-
Notifications
You must be signed in to change notification settings - Fork 168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added Power block and generalized Log block #2221
Added Power block and generalized Log block #2221
Conversation
Thank you very much for this PR. The comment of Power.base should better read Base of power. |
|
|
One more issue (also raised by @HansOlsson) is if the natural base should be considered explicitly, such that pow(e, u) maps to exp(u) and log(u)/log(e) maps to lo log(u). |
How should "base is equal to e" be implemented? Check for a small delta deviation? How big shall the delta deviation be? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Log description still says:
"Output the natural (base e) logarithm of the input (input > 0 required)"
This is no longer correct, I would prefer something like:
"Output the logarithm (default base e) of the input (input > 0 required)"
And similarly in the documentation.
That is a tricky question, but assuming For the exponent the issue is that e.g. The relative difference is 1e-15, i.e. a factor 10 above round-off - and exponentiation gives the right answer, to obtain the right answer in that case we could use The underlying reason is that log, exp, pow in modern C-implementations use infinite precision and then round. |
@HansOlsson As Modelica.Math.log(base)==1 seems to give a pretty clear answer on the base being equal to e, why can't we just use the following code: block Log "Output the logarithm (default base e) of the input (input > 0 required)"
extends Interfaces.SISO;
parameter Real base = Modelica.Constants.e "Base of logarithm" annotation(Evaluate=true);
equation
y = if Modelica.Math.log(base)==1 then Modelica.Math.log(u) else Modelica.Math.log(u)/Modelica.Math.log(base);
end Log;
block Power "Output the power to a base of the input"
extends Interfaces.SISO;
parameter Real base = Modelica.Constants.e "Base of power" annotation(Evaluate=true);
equation
y = if Modelica.Math.log(base)==1 then Modelica.Math.exp(u) else base ^ u;
end Power; If you do not agree, could you please provide an alternative code snippet. |
The documentation already reads:
|
I was thinking of an extra enumeration type for the base, either natural or user input. |
For log just write (since division by 1 is exact):
For Power it is cleaner if we just write
Except that also this variant has equality-comparison between reals which isn't legal, so we have to use We could let the user decided (along the lines suggested by @tbeu ):
I am unsure if that difference is significant enough. |
I think the Log block is quite good as @HansOlsson proposed it. Considering the options of the Exp block I think that two options make sense:
|
Looks good to me. Thank you for your contribution! Going to merge it soon. |
Squashed and applied. |
I realize that it's a bit late to have comments on a closed pull request but this block occupies the generic name Power, what about the |
It is not too late as long as it was not yet released. |
See #2191